Skip to content

Conversation

averissimo
Copy link
Contributor

@averissimo averissimo commented Jun 3, 2025

@averissimo averissimo marked this pull request as ready for review June 5, 2025 13:11
Copy link
Contributor

github-actions bot commented Jun 5, 2025

badge

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  --------------------------------------
R/tm_a_pca.R                    876     876  0.00%    141-1146
R/tm_a_regression.R             765     765  0.00%    180-1044
R/tm_data_table.R               201     201  0.00%    100-349
R/tm_file_viewer.R              172     172  0.00%    47-254
R/tm_front_page.R               144     133  7.64%    77-247
R/tm_g_association.R            327     327  0.00%    161-557
R/tm_g_bivariate.R              686     422  38.48%   332-811, 852, 963, 980, 998, 1009-1031
R/tm_g_distribution.R          1100    1100  0.00%    158-1400
R/tm_g_response.R               355     355  0.00%    179-609
R/tm_g_scatterplot.R            720     720  0.00%    261-1081
R/tm_g_scatterplotmatrix.R      283     264  6.71%    200-516, 577, 591
R/tm_missing_data.R            1095    1095  0.00%    126-1402
R/tm_outliers.R                1022    1022  0.00%    165-1337
R/tm_t_crosstable.R             269     269  0.00%    177-491
R/tm_variable_browser.R         803     798  0.62%    89-1044, 1082-1265
R/utils.R                       142     126  11.27%   87-272, 301-327, 339-348, 353, 367-386
R/zzz.R                           2       2  0.00%    2-3
TOTAL                          8962    8647  3.51%

Diff against main

Filename                      Stmts    Miss  Cover
--------------------------  -------  ------  --------
R/tm_a_pca.R                    -13     -13  +100.00%
R/tm_a_regression.R              -8      -8  +100.00%
R/tm_g_association.R            -17     -17  +100.00%
R/tm_g_bivariate.R              -12     -12  +0.66%
R/tm_g_distribution.R           -17     -17  +100.00%
R/tm_g_response.R               -14     -14  +100.00%
R/tm_g_scatterplot.R            -14     -14  +100.00%
R/tm_g_scatterplotmatrix.R      -14     -14  +0.32%
R/tm_missing_data.R             -24     -24  +100.00%
R/tm_outliers.R                 -23     -23  +100.00%
R/tm_t_crosstable.R             -16     -16  +100.00%
R/utils.R                        -9      -9  +0.67%
TOTAL                          -181    -181  +0.07%

Results for commit: 7c2dc13

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Jun 5, 2025

Unit Tests Summary

  1 files  22 suites   2s ⏱️
144 tests 29 ✅ 115 💤 0 ❌
182 runs  67 ✅ 115 💤 0 ❌

Results for commit 7c2dc13.

♻️ This comment has been updated with latest results.

m7pr and others added 2 commits June 6, 2025 14:26
Companion to
insightsengineering/teal.reporter#334
Consequence of changing naming convention for `teal_report` object.

---------

Co-authored-by: André Veríssimo <[email protected]>
averissimo added a commit to insightsengineering/teal.code that referenced this pull request Jun 12, 2025
…l_data` (#255)

# Pull Request

Fixes:

- insightsengineering/teal#1526

Built on top of:

- insightsengineering/teal.reporter#307
    - _(#307 will be closed once this PR is stable)_

### Companion PRs:

- insightsengineering/teal#1541
- #255
- insightsengineering/teal.data#370
- insightsengineering/teal.reporter#331
- insightsengineering/teal.modules.general#884

### Changes description

- [x] Add new parameter `cache`
- Caches the result of the last evaluation in the respective `@code`
slot
    - [ ] Decide on name
- [x] Remove signature with multiple arguments to allow overriding
`eval_code` in other packages without showing a note

``` r
pkgload::load_all("teal.code")
#> ℹ Loading teal.code

q <- qenv() |> 
  eval_code(1 + 1, cache = TRUE) |> 
  eval_code(mtcars <- head(mtcars))

attr(q@code[[1]], "cache")
#> [1] 2
```

<sup>Created on 2025-06-03 with [reprex
v2.1.1](https://reprex.tidyverse.org)</sup>

---------

Co-authored-by: Dawid Kaledkowski <[email protected]>
Co-authored-by: Marcin <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
averissimo added a commit to insightsengineering/teal.data that referenced this pull request Jun 12, 2025
…l_data` (#370)

# Pull Request

Fixes:

- insightsengineering/teal#1526

Built on top of:

- insightsengineering/teal.reporter#307
    - _(#307 will be closed once this PR is stable)_

### Companion PRs:

- insightsengineering/teal#1541
- insightsengineering/teal.code#255
- #370
- insightsengineering/teal.reporter#331
- insightsengineering/teal.modules.general#884

### Changes description

- [x] Cleanup of `teal_data` class to allow for `teal_report` extension
- [x] Change the `show()` method to remove reference to `teal_data`
specifically

---------

Co-authored-by: Dawid Kaledkowski <[email protected]>
Co-authored-by: Marcin <[email protected]>
averissimo and others added 2 commits June 17, 2025 14:42
# Pull Request

- Changes from `teal_reportable` branch at `{ŧeal.report}`
- Remove old logic that was defusing the argument
plot_top <- plots[[1]]
plot_bottom <- plots[[2]]
plot <- gridExtra::grid.arrange(plot_top, plot_bottom, ncol = 1)
plot <- gridExtra::arrangeGrob(plots[[1]], plots[[2]], ncol = 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused arguments (list(), "Race [RACE]") 
 when evaluating qenv code:
plot <- gridExtra::arrangeGrob(plots[[1]], plots[[2]], ncol = 1)

@m7pr
Copy link
Contributor

m7pr commented Aug 26, 2025

What's done

  • merged main to this branch
  • fixed installation of teal and teal.reporter on teal_reportable branch
  • changed eval_codes from 'library()' calls to expression(library calls so there is less quotes
  • tested all modules with this app App with all modules for testing #913
  • it is possible to run the app and add cards to the reporter for all modules besides one

Next steps

Comment on lines +493 to +508
reactive({
# Version 1
# since data() is not being build up in this module, we need to create a reactive that will
# return the plot in the reactive teal_data()
validation_checks()
obj <- data()
env <- new.env()
assign("plot", variable_plot_r(), envir = env)
[email protected] <- rlang::env_clone(env)
teal.reporter::teal_card(obj) <-
c(
teal.reporter::teal_card("# Variable Browser Plot"),
teal.reporter::teal_card(obj),
teal.reporter::teal_card("## Module's code")
)
teal.code::eval_code(obj, "plot")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @averissimo this module is harder as it does not build up any code to show R code. originally it only showed the plot like in Version 2 below.

With version 1 we are able to show libraries call, and filtering code, but not the plot code and it looks odd. Also I needed to overwrite @.xData which is extremely dirty.

Do you think we rather support Version 1 or Version 2 for this module?
image

Comment on lines +511 to +513
# VERSION 2
# validation_checks()
# teal.data::teal_data(plot = variable_plot_r()) |> teal.code::eval_code("plot")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image


anl_merged_q <- reactive({
req(anl_merged_input())
qenv() %>%
teal.code::eval_code(as.expression(anl_merged_input()$expr)) %>%
teal.code::eval_code(quote(ANL)) # used to display table when running show-r-code code
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

})
)

decorated_summary_table_q <- srv_decorate_teal_data(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not used later at all

card
reactive(
if (input$tabs == "Histogram") {
c(decorated_output_dist_q(), decorated_output_summary_q(), decorated_output_test_q())
Copy link
Contributor

@m7pr m7pr Aug 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the c gives a warning - maybe we can add suppressWarning in here?

image
  1. do we need headers, for Statistics table and Tests table?

Now it's just 2 tables under the plot
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants